-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Port MASTG-TEST-0045: Testing Root Detection (android) #3136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The proposal here focuses on a demo which illustrates some basic checks that can be implemented to detect root, but the purpose of the TEST is to test whether root devices are detected by an app.
I'm not sure the usefulness of the DEMO in that regard. If someone implements these basic checks, I think they will not meaningfully address the weakness.
I think the TEST should remain focused on what good root detection characteristics are. (resilience to dynamic bypasses, breadth of root scenarios detected, effectiveness of detections, as described in the TEST).
the other valuable contribution to this TEST would be examples of how to test for those characteristics. e.g. how to setup a wide enough range of root scenarios, how to test bypassing a root detection, etc..
Thanks for the PR @martinzigrai. The new MASTG tests aim at consistency (in code, content, language and structure) and reproducibility, so, before we start with the actual review, please make sure that everything is in line with our guidelines and other recently added demos (go here and sort the list, the highest numbers indicate more recent demos). For example:
Please check the latest tests and demos and read the following: If you have any questions, I'll be happy to help. Thank you very much! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a couple of suggestions to make it more consistent with the rest of the v2.
demos/android/MASVS-RESILIENCE/MASTG-DEMO-0033/MASTG-DEMO-0033.md
Outdated
Show resolved
Hide resolved
Hi @serek8, Thanks a lot for the feedback. The suggestions are very substantive and really improve the clarity and consistency. I'll apply all of them. Thanks! |
Co-authored-by: Jan Seredynski <janseredynski@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the submission! Just two small comments :)
|
||
The testing process involves analyzing the device environment to identify common indicators of root access. This includes checking for the presence of: | ||
|
||
- root management tools - e.g. Magisk, SuperSU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is SuperSU still relevant? Maybe we should go for Magisk, KernelSU ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the feedback. It is a valid point, and I will update the pull request accordingly.
|
||
### Evaluation | ||
|
||
The test fails because the app relies on only one root detection method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output.txt does indeed only include one hit, but the application checks for root using Runtime.exec and File.exists. So the detection rule should be improved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a fair question. The detection rule is actually working as intended. It uses a pattern-either logic, so it's designed to report success as soon as it finds the first match in the code, which is why you only see one hit.
This PR closes #3021